Skip to content

Conversation

keith-packard
Copy link

This selects picolibc by using --specs=picolibc.specs for compiling and linking. The linker scripts are adjusted to add KEEP to ensure that --gc-sections doesn't remove the contents of the files.

Copy link
Member

@soburi soburi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wondering, how should it behave if CONFIG_PICOLIBC is not selected?

Usually, we use PICOLIBC, so I think it's generally fine.

target_link_options(${NAME} PRIVATE "-nostdlib")
elseif (PICO_C_COMPILER_IS_GNU)
target_link_options(${NAME} PRIVATE "--specs=nosys.specs")
target_compile_options(${NAME} PRIVATE "--specs=picolibc.specs")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a history about this change to ChangeLog.zephyr.md.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, done.

This selects picolibc by using --specs=picolibc.specs for compiling
and linking. The linker scripts are adjusted to add KEEP to ensure
that --gc-sections doesn't remove the contents of the files.

Signed-off-by: Keith Packard <[email protected]>
@keith-packard
Copy link
Author

Just wondering, how should it behave if CONFIG_PICOLIBC is not selected?

The boot loader can use picolibc even if the app needs newlib for some reason; we still get the picolibc benefits in the boot loader.


target_link_libraries(${NAME} hardware_regs boot_stage2_headers)
target_link_options(${NAME} PRIVATE "LINKER:--script=${PICO_BOOT_STAGE2_DIR}/boot_stage2.ld")
target_link_options(${NAME} PRIVATE "-T${PICO_BOOT_STAGE2_DIR}/boot_stage2.ld")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a minor point, but I think long options are easier to read in scripts.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

picolibc's spec fragments only look for the -T version of this option, so we must use the short variant to prevent addition of the picolibc linker script here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I got it.
Thank you for your clarification.

@soburi soburi self-assigned this Jun 26, 2025
@soburi soburi requested a review from ajf58 June 26, 2025 21:27
@soburi
Copy link
Member

soburi commented Jun 26, 2025

@ajf58 @ThreeEights

Could you take a look, please?

@ajf58
Copy link
Collaborator

ajf58 commented Jun 26, 2025

Just wondering, how should it behave if CONFIG_PICOLIBC is not selected?

The boot loader can use picolibc even if the app needs newlib for some reason; we still get the picolibc benefits in the boot loader.

Can you expand on this? What's the benefit? I've read the commit message and it has the what, but not the why of this.

@keith-packard
Copy link
Author

Can you expand on this? What's the benefit? I've read the commit message and it has the what, but not the why of this.

Probably the most visible benefit is a reduction in memory usage -- picolibc has smaller implementations of many core parts of the C library (stdio, ctype, locales) as well as general memory saving architectural changes (eliminating struct reent).

It's also routinely tested for the target systems. I'll be enabling these tests within the Zephyr SDK build to catch bugs introduced in any part of the system (binutils, compiler, linker, libraries). Newlib lacks any visible CI system.

Copy link
Collaborator

@ajf58 ajf58 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation/rationale. It's a bit of a nit to say I'd prefer to see it in the commit message, but it's here in the PR instead.

@soburi soburi merged commit b547a36 into zephyrproject-rtos:zephyr Jun 27, 2025
4 checks passed
@soburi
Copy link
Member

soburi commented Jun 27, 2025

@keith-packard

Merged. Please update west.yml.

@soburi
Copy link
Member

soburi commented Sep 9, 2025

@keith-packard

This commit has not yet been reflected in west.yml,
but judging from the history of this zephyrproject-rtos/zephyr#93470, it appears to have been dropped.
Is it no longer needed?

@keith-packard
Copy link
Author

@keith-packard

This commit has not yet been reflected in west.yml, but judging from the history of this zephyrproject-rtos/zephyr#93470, it appears to have been dropped. Is it no longer needed?

west.yml got this patch when it pulled in some other hal changes.

@soburi
Copy link
Member

soburi commented Sep 10, 2025

@keith-packard
This commit has not yet been reflected in west.yml, but judging from the history of this zephyrproject-rtos/zephyr#93470, it appears to have been dropped. Is it no longer needed?

west.yml got this patch when it pulled in some other hal changes.

I see. I didn't get a notification about that, so I missed it. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants